-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Porting test_tf2 #203
Porting test_tf2 #203
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite a number of things here, though many of them are small things to fix.
5f382d3
to
7e5d582
Compare
…/test/test_tf2
9ceb430
to
740a310
Compare
@clalancette finally CI is green. We can merge #217 and #216 too All this 3 PR are included and tested in this branch https://github.com/ros2/geometry2/tree/ahcorde/test/test_tf2_msg_filter_fix2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right, quite a few more comments. However, they are all small problems to be fixed either in comments, include files, or with removing dead code. We won't need a full CI run for these; just on one platform should ensure that nothing broke.
|
||
*/ | ||
|
||
// /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is still open.
test_tf2/test/buffer_core_test.cpp
Outdated
} | ||
|
||
// rclcpp::shutdown(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is still open.
test_tf2/test/buffer_core_test.cpp
Outdated
{ | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should just remove this commented-out code.
test_tf2/test/buffer_core_test.cpp
Outdated
{ | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this commented out code.
test_tf2/test/buffer_core_test.cpp
Outdated
{ | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); | ||
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this commented out code.
test_tf2/test/buffer_core_test.cpp
Outdated
@@ -2809,7 +2868,7 @@ TEST(tf2_stamped, OperatorEqual) | |||
*/ | |||
int main(int argc, char **argv){ | |||
testing::InitGoogleTest(&argc, argv); | |||
builtin_interfaces::msg::Time::init(); //needed for builtin_interfaces::msg::Time::now() | |||
ros::init(argc, argv, "tf_unittest"); | |||
// builtin_interfaces::msg::Time::init(); //needed for builtin_interfaces::msg::Time::now() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented-out code.
test_tf2/test/permuter.hpp
Outdated
#include <vector> | ||
#include <mutex> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: alphabetize.
test_tf2/test/test_buffer_client.cpp
Outdated
#include <chrono> | ||
#include <thread> | ||
#include <functional> | ||
#include <memory> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: alphabetize.
#include <memory> | ||
#include <functional> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: alphabetize.
}; | ||
|
||
TEST(StaticTransformPublisher, tf_from_param_server_valid) | ||
// TODO (ahcorde) Load transform from yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand a bit on what needs to be done to port this test?
The code changes now look good to me. Do you know what is going on with the test failure in CI? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good assuming green CI.
Initial port for the package test_tf2